fix(webview): throttle state pushes to prevent gray screen OOM - #1077
Closed
JunyongParkDev wants to merge 4 commits into
Closed
fix(webview): throttle state pushes to prevent gray screen OOM#1077JunyongParkDev wants to merge 4 commits into
JunyongParkDev wants to merge 4 commits into
Conversation
JunyongParkDev
requested review from
JamesRobert20,
edelauna,
hannesrudolph,
navedmerchant and
taltas
as code owners
July 31, 2026 15:26
Contributor
📝 WalkthroughWalkthroughTask state pushes now use throttled provider methods with explicit flushes for partial messages, unanswered asks, and aborts. Provider disposal cancels pending updates. ChatView now resets and renders aggregated costs by current task. ChangesTask webview state and cost handling
Estimated code review effort: 3 (Moderate) | ~25 minutes Sequence Diagram(s)sequenceDiagram
participant Task
participant ClineProvider
participant Webview
participant ChatView
Task->>ClineProvider: schedule throttled task state
ClineProvider->>Webview: post coalesced state
Webview->>ChatView: deliver task state and aggregated-cost events
ChatView->>ChatView: accept data for currentTaskId
Task->>ClineProvider: flush pending state before partial update or abort event
ClineProvider->>Webview: post pending state
Possibly related issues
Possibly related PRs
Suggested labels: Suggested reviewers: 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
Codecov Report✅ All modified and coverable lines are covered by tests. 📢 Thoughts on this report? Let us know! |
Coalesce repeated full-state webview updates with a leading and trailing debounce and a one-second maximum wait. Keep task-start, API-boundary, and stream-completion updates immediate, and flush pending state during partial-message initialization and task abort. Reset aggregated task costs when switching tasks and ignore delayed responses for inactive tasks so stale per-task data is not retained or displayed. Add regression coverage for debounce timing, flush and disposal behavior, partial-message ordering, queue failures, and task-switch cost cleanup. Signed-off-by: JunyongParkDev <jun94.park@samsung.com>
Verify stale webview messages are cleared and posted through the immediate state path before the first task message. This keeps task startup outside the throttled update path. Signed-off-by: JunyongParkDev <jun94.park@samsung.com>
Ensure unanswered ask state reaches the webview before Message listeners can respond. Keep already answered asks on the throttled path and cover both ordering cases with regression tests. Signed-off-by: JunyongParkDev <jun94.park@samsung.com>
Exercise the stringification branch for non-Error rejections so both debounced state-post failure paths are covered. Signed-off-by: JunyongParkDev <jun94.park@samsung.com>
JunyongParkDev
force-pushed
the
fix/webview-state-throttling
branch
from
July 31, 2026 16:27
8156406 to
11be8e5
Compare
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@src/core/task/__tests__/Task.spec.ts`:
- Around line 1698-1734: Add rejection-path tests for addToClineMessages and
abortTask, using rejected throttled provider calls to verify
RooCodeEventName.Message and RooCodeEventName.TaskAborted behavior remains
observable as appropriate. Cover disposal’s flush path as needed, asserting
errors are logged or otherwise handled and that the rejections do not escape or
prevent completion of the surrounding operation.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: 5a4ab539-0185-4b98-be94-9cd2849185a2
📒 Files selected for processing (7)
src/core/task/Task.tssrc/core/task/__tests__/Task.spec.tssrc/core/task/__tests__/Task.throttle.test.tssrc/core/webview/ClineProvider.tssrc/core/webview/__tests__/ClineProvider.spec.tswebview-ui/src/components/chat/ChatView.tsxwebview-ui/src/components/chat/__tests__/ChatView.spec.tsx
🚧 Files skipped from review as they are similar to previous changes (6)
- src/core/task/tests/Task.throttle.test.ts
- src/core/task/Task.ts
- src/core/webview/tests/ClineProvider.spec.ts
- webview-ui/src/components/chat/tests/ChatView.spec.tsx
- src/core/webview/ClineProvider.ts
- webview-ui/src/components/chat/ChatView.tsx
Comment on lines
+1698
to
+1734
| it("waits for an unanswered ask flush before emitting the message", async () => { | ||
| const task = new Task({ | ||
| provider: mockProvider, | ||
| apiConfiguration: mockApiConfig, | ||
| task: "test task", | ||
| startTask: false, | ||
| }) | ||
| const taskAccess = getTaskTestAccess(task) | ||
| vi.spyOn(taskAccess, "saveClineMessages").mockResolvedValue(true) | ||
| let releaseFlush!: () => void | ||
| const pendingFlush = new Promise<void>((resolve) => { | ||
| releaseFlush = resolve | ||
| }) | ||
| const flushSpy = vi.mocked(mockProvider.flushPostStateToWebviewThrottled).mockReturnValueOnce(pendingFlush) | ||
| const messageListener = vi.fn() | ||
| task.on(RooCodeEventName.Message, messageListener) | ||
| const message = { | ||
| ts: 1, | ||
| type: "ask" as const, | ||
| ask: "resume_task" as const, | ||
| } | ||
|
|
||
| const addPromise = taskAccess.addToClineMessages(message) | ||
|
|
||
| await Promise.resolve() | ||
| expect(mockProvider.postStateToWebviewThrottled).toHaveBeenCalledOnce() | ||
| expect(mockProvider.postStateToWebviewThrottled).toHaveBeenCalledWith() | ||
| expect(flushSpy).toHaveBeenCalledOnce() | ||
| expect(flushSpy).toHaveBeenCalledWith() | ||
| expect(messageListener).not.toHaveBeenCalled() | ||
|
|
||
| releaseFlush() | ||
| await addPromise | ||
|
|
||
| expect(flushSpy.mock.invocationCallOrder[0]).toBeLessThan(messageListener.mock.invocationCallOrder[0]) | ||
| expect(messageListener).toHaveBeenCalledWith({ action: "created", message }) | ||
| }) |
Contributor
There was a problem hiding this comment.
🩺 Stability & Availability | 🟠 Major | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== locate files =="
git ls-files | rg '(^|/)Task\.ts$|(^|/)Task\.spec\.ts$' || true
echo "== Task.spec relevant ranges =="
for f in $(git ls-files | rg 'Task\.spec\.ts$'); do
echo "--- $f lines 1650-1900"
sed -n '1650,1900p' "$f" | nl -ba -v1650
done
echo "== Task.ts outline/search around candidate symbols =="
for f in $(git ls-files | rg '^(src/core/task)/Task\.ts$|Task\.ts$'); do
echo "--- $f"
wc -l "$f"
ast-grep outline "$f" --match addToClineMessages --view expanded || true
ast-grep outline "$f" --match abortTask --view expanded || true
ast-grep outline "$f" --match messageQueueStateChangedHandler --view expanded || true
echo "--- postState/flush occurrences"
rg -n "postStateToWebviewThrottled|flushPostStateToWebviewThrottled|messageQueueStateChangedHandler|TaskAborted|dispose\(" "$f" -C 4
doneRepository: Zoo-Code-Org/Zoo-Code
Length of output: 367
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== Task.spec relevant ranges =="
for f in $(git ls-files | rg 'Task\.spec\.ts$'); do
echo "--- $f lines 1650-1900"
sed -n '1650,1900p' "$f" | cat -n
done
echo "== Task.ts candidate occurrences =="
f="src/core/task/Task.ts"
echo "--- $f line count"
wc -l "$f"
echo "--- postState/flush/messageQueue occurrences"
grep -n "postStateToWebviewThrottled\|flushPostStateToWebviewThrottled\|messageQueueStateChangedHandler\|TaskAborted\|dispose(" "$f" -C 8 || true
echo "== addTask/messageQueue abort-like handlers in Task.ts =="
# Use Python to print concise named function blocks if possible without heavy runtime.
python3 - <<'PY'
from pathlib import Path
p=Path('src/core/task/Task.ts')
text=p.read_text()
for needle in ['addToClineMessages', 'messageQueueStateChangedHandler', 'abortTask']:
print(f'---- {needle} ----')
i=text.find(needle)
while i!=-1:
start=max(0,i-800)
end=min(len(text),i+2200)
print(text[start:end])
print('----')
i=text.find(needle,end)
PYRepository: Zoo-Code-Org/Zoo-Code
Length of output: 46233
Add rejection-path coverage for the throttled provider calls.
addToClineMessages awaits postStateToWebviewThrottled() before emitting RooCodeEventName.Message. If flushPostStateToWebviewThrottled() rejects, execution stops before this.emit(RooCodeEventName.Message, ...) and saveClineMessages(). abortTask awaits flushPostStateToWebviewThrottled() before emitting RooCodeEventName.TaskAborted, and disposal only wraps dispose()/saveClineMessages(), so a rejection can prevent post-state flush completion and expose an unwrapped rejection before abort completion.
Add tests in this block, and a rejection-path test near the abort tests, that assert the logged or observable behavior and do not let these rejections propagate.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/core/task/__tests__/Task.spec.ts` around lines 1698 - 1734, Add
rejection-path tests for addToClineMessages and abortTask, using rejected
throttled provider calls to verify RooCodeEventName.Message and
RooCodeEventName.TaskAborted behavior remains observable as appropriate. Cover
disposal’s flush path as needed, asserting errors are logged or otherwise
handled and that the rejections do not escape or prevent completion of the
surrounding operation.
8 tasks
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Related GitHub Issue
Closes: #629
Description
This PR reduces full-state webview churn during long-running tasks with large message histories. The hot paths previously serialized and posted the complete message array for every message addition and queue-state change, producing repeated multi-megabyte snapshots at the message count reported in #629.
Partial streaming continues to use the existing lightweight
messageUpdatedpath. The change coalesces high-frequency full snapshots without changing the webview message protocol or task lifecycle. Reviewers should pay particular attention to full-state/lightweight-message ordering and the rejection paths around persistence and abort cleanup.Test Procedure
Run the focused regression tests:
Run the repository validation commands:
pnpm test pnpm check-types pnpm lintLocal results:
The regression tests cover:
messageUpdateddelivery.Manual stress verification:
Pre-Submission Checklist
Visual Snapshots
Not applicable. This change does not alter static visual output.
Videos (interaction / animation only)
Not applicable. This change does not add or alter a visible interaction or animation.
Documentation Updates
Additional Notes
This PR intentionally addresses the frequency of large full-state pushes. Redesigning message delivery to eliminate the O(N) payload itself is outside the scope of #629.
The manual stress run verified webview renderer survival and responsiveness at the reported message count. It did not record an absolute V8 heap measurement.
Get in Touch